Fix aspect ratio when stretching with main axis margin #834
Conversation
|
Thank you for your pull request. We require contributors to sign our Contributor License Agreement, and yours has expired. Before we can review or merge your code, we need you to email cla@fb.com with your details so we can update your status. |
|
Ok the test I added is passing now.I noticed that this is not just an issue with However I was not able to create a failing test for this, so my guess is that this was probably fixed previously and the yoga version used in the layout editor just hasn't been updated yet. Would like to confirm this though. |
|
The yoga used in playground has not been updated since a long time. There are few bugs which were fixed and still its reproducible on playground. |
|
@priteshrnandgaonkar yeah that was my guess |
emilsjolander
changed the title
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
There was a problem hiding this comment.
@davidaurelio has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.
|
Nice catch! This fix doesn't seem right though (being constrained to just stretch) -- are you still working on a better fix or do you think this is all that's needed? |
|
@astreet I think this is all that's needed. The fix is specifically for stretch as that is the case where it needs to be set as an exact measurement. Not sure what else would need to be covered by this. |
Summary: I've noticed that when a child's size is determined by `align-items: stretch` in combination with `aspect-ratio` its size is wrongly calculated to account for margin in the main axis when there is more than enough space. See playground: https://goo.gl/tgW6cD I've yet to figure out exactly how to solve this but i've started by writing a failing test when can be seen in the first commit here. I assumed I had found the bug here https://github.com/facebook/yoga/blob/master/yoga/Yoga.cpp#L1838 where margin is being subtracted from the desired width even though the measure mode tells it to measure to exactly that size. However, if we don't remove this margin from the available width then 15 tests fail (including the one I just added) not quite figured out why yet. I'm also a bit confused at to why this would only happen for nodes with `aspect-ratio` and not for nodes where an explicit height and width is set. Pull Request resolved: #834 Reviewed By: astreet Differential Revision: D13223579 Pulled By: davidaurelio fbshipit-source-id: 6970e6072e79f3bb6f9097355ab6e441441bfd88
Summary: I've noticed that when a child's size is determined by `align-items: stretch` in combination with `aspect-ratio` its size is wrongly calculated to account for margin in the main axis when there is more than enough space. See playground: https://goo.gl/tgW6cD I've yet to figure out exactly how to solve this but i've started by writing a failing test when can be seen in the first commit here. I assumed I had found the bug here https://github.com/facebook/yoga/blob/master/yoga/Yoga.cpp#L1838 where margin is being subtracted from the desired width even though the measure mode tells it to measure to exactly that size. However, if we don't remove this margin from the available width then 15 tests fail (including the one I just added) not quite figured out why yet. I'm also a bit confused at to why this would only happen for nodes with `aspect-ratio` and not for nodes where an explicit height and width is set. Pull Request resolved: facebook/yoga#834 Reviewed By: astreet Differential Revision: D13223579 Pulled By: davidaurelio fbshipit-source-id: 6970e6072e79f3bb6f9097355ab6e441441bfd88
Summary: I've noticed that when a child's size is determined by `align-items: stretch` in combination with `aspect-ratio` its size is wrongly calculated to account for margin in the main axis when there is more than enough space. See playground: https://goo.gl/tgW6cD I've yet to figure out exactly how to solve this but i've started by writing a failing test when can be seen in the first commit here. I assumed I had found the bug here https://github.com/facebook/yoga/blob/master/yoga/Yoga.cpp#L1838 where margin is being subtracted from the desired width even though the measure mode tells it to measure to exactly that size. However, if we don't remove this margin from the available width then 15 tests fail (including the one I just added) not quite figured out why yet. I'm also a bit confused at to why this would only happen for nodes with `aspect-ratio` and not for nodes where an explicit height and width is set. Pull Request resolved: facebook/yoga#834 Reviewed By: astreet Differential Revision: D13223579 Pulled By: davidaurelio fbshipit-source-id: 6970e6072e79f3bb6f9097355ab6e441441bfd88
I've noticed that when a child's size is determined by
align-items: stretchin combination withaspect-ratioits size is wrongly calculated to account for margin in the main axis when there is more than enough space.See playground: https://goo.gl/tgW6cD
I've yet to figure out exactly how to solve this but i've started by writing a failing test when can be seen in the first commit here.
I assumed I had found the bug here https://github.com/facebook/yoga/blob/master/yoga/Yoga.cpp#L1838 where margin is being subtracted from the desired width even though the measure mode tells it to measure to exactly that size. However, if we don't remove this margin from the available width then 15 tests fail (including the one I just added) not quite figured out why yet. I'm also a bit confused at to why this would only happen for nodes with
aspect-ratioand not for nodes where an explicit height and width is set.